Skip to content

BUG: fix memory_usage method with deep of StringArray #33985

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged

Conversation

kotamatsuoka
Copy link
Contributor

@kotamatsuoka kotamatsuoka force-pushed the add-memory-usage-method-to-StringArray branch 3 times, most recently from a99b701 to d0d94fb Compare May 5, 2020 03:54
@kotamatsuoka kotamatsuoka force-pushed the add-memory-usage-method-to-StringArray branch from d0d94fb to fcb4de1 Compare May 5, 2020 05:35
@jorisvandenbossche jorisvandenbossche added the ExtensionArray Extending pandas with custom dtypes or arrays. label May 5, 2020
@@ -85,6 +85,14 @@ class TestMethods(base.BaseMethodsTests):
def test_value_counts(self, all_data, dropna):
return super().test_value_counts(all_data, dropna)

def test_memory_usage(self, data):
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you move this test to /tests/arrays/string_/test_string.py ?

(the tests in /extension/ are all common tests (shared among dtypes), while in /arrays/ we keep specific tests)

@jorisvandenbossche jorisvandenbossche changed the title BUG: memory_usage method with deep of StringArray is wrong BUG: fix memory_usage method with deep of StringArray May 5, 2020
@jorisvandenbossche
Copy link
Member

@kotamatsuoka thanks a lot for working on this!

Copy link
Contributor

@jreback jreback left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can you add a note in bugfix section, ExtensionArray in 1.1

@kotamatsuoka kotamatsuoka force-pushed the add-memory-usage-method-to-StringArray branch from 3edc7ae to 697d98e Compare May 5, 2020 12:46
@jreback jreback added this to the 1.1 milestone May 5, 2020
@jreback jreback merged commit de8ca78 into pandas-dev:master May 5, 2020
@jreback
Copy link
Contributor

jreback commented May 5, 2020

thanks @kotamatsuoka

as a followup can you add tests for each EA that we have. note that it is possible for memory usage under deep=True to be equal to deep=False (for some implementations).

@kotamatsuoka kotamatsuoka deleted the add-memory-usage-method-to-StringArray branch May 5, 2020 14:33
jbrockmendel pushed a commit to jbrockmendel/pandas that referenced this pull request May 6, 2020
rhshadrach pushed a commit to rhshadrach/pandas that referenced this pull request May 10, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug ExtensionArray Extending pandas with custom dtypes or arrays.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

BUG: memory_usage(deep=True) of dtype='string' is wrong
3 participants